Add pygame.display.get_desktop_usable_bounds#3530
Add pygame.display.get_desktop_usable_bounds#3530damusss wants to merge 10 commits intopygame-community:mainfrom
pygame.display.get_desktop_usable_bounds#3530Conversation
damusss
added
New API
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughAdds pygame.display.get_desktop_usable_bounds(), a new C-implemented function that returns a list of pygame.Rect representing usable desktop areas (excluding OS-occupied regions) per virtual desktop; updates stub typing, adds a docs macro, and includes a unit test validating types and size constraints. Changes
Sequence Diagram(s)sequenceDiagram
participant Py as Python caller
participant Display as pygame.display
participant C as pg_get_desktop_usable_bounds (C)
participant SDL as SDL Video API
Py->>Display: get_desktop_usable_bounds()
Display->>C: call (no args)
alt SDL3
C->>SDL: SDL_GetDisplays()\nSDL_GetDisplayUsableBounds(display)
else pre-SDL3
C->>SDL: SDL_GetNumVideoDisplays()\nSDL_GetDisplayUsableBounds(index)
end
SDL-->>C: usable bounds per display
C-->>Display: list[Rect]
Display-->>Py: list[Rect]
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/display_test.py (1)
700-709: Avoid potential IndexError and assert list alignmentIf, for any reason, the two lists differ in length, this loop will raise IndexError. Assert the lengths match and iterate with zip to keep the test robust.
Apply this diff:
def test_get_desktop_usable_bounds(self): bounds = pygame.display.get_desktop_usable_bounds() sizes = pygame.display.get_desktop_sizes() self.assertIsInstance(bounds, list) - for i, bound in enumerate(bounds): - self.assertIsInstance(bound, pygame.Rect) - size = sizes[i] - self.assertLessEqual(bound.w, size[0]) - self.assertLessEqual(bound.h, size[1]) + self.assertEqual(len(bounds), len(sizes)) + for bound, size in zip(bounds, sizes): + self.assertIsInstance(bound, pygame.Rect) + self.assertLessEqual(bound.w, size[0]) + self.assertLessEqual(bound.h, size[1])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
buildconfig/stubs/pygame/display.pyi(2 hunks)src_c/display.c(2 hunks)src_c/doc/display_doc.h(1 hunks)test/display_test.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
test/display_test.py (2)
buildconfig/stubs/pygame/display.pyi (2)
get_desktop_usable_bounds(425-441)get_desktop_sizes(406-423)buildconfig/stubs/pygame/rect.pyi (1)
Rect(303-303)
buildconfig/stubs/pygame/display.pyi (4)
buildconfig/stubs/pygame/typing.pyi (1)
rect(57-57)src_py/typing.py (1)
rect(57-57)buildconfig/stubs/typing_sample_app.py (1)
rect(100-101)buildconfig/stubs/pygame/rect.pyi (1)
Rect(303-303)
🔇 Additional comments (4)
src_c/doc/display_doc.h (1)
14-14: Doc macro addition LGTMThe new macro name and summary align with existing patterns and the API behavior.
buildconfig/stubs/pygame/display.pyi (2)
425-442: Stub signature and docstring LGTMReturn type list[Rect] matches runtime objects. The docstring mirrors the new API semantics and version tag is present.
55-55: No Union usage detected – import removal is safe
- buildconfig/stubs/pygame/display.pyi: scanned for “Union” with ripgrep; no occurrences found.
src_c/display.c (1)
3179-3181: Method registration LGTMCorrectly registered with METH_NOARGS and linked to the doc macro.
|
This could be used to implement "Do what I mean" window sizing when passing in (0,0) for windowed mode. I always hated how windowed mode overlaps with the task bar in that case! |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
MyreMylar
left a comment
MyreMylar
left a comment
There was a problem hiding this comment.
OK, LGTM 👍 Seems like it could be useful for application positioning especially with things like no frame windows.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/display_test.py (1)
700-710: LGTM! Consider optional enhancements for more comprehensive coverage.The test correctly validates the return type, length consistency with
get_desktop_sizes(), and basic dimension constraints.For enhanced test coverage, you may optionally consider:
- Validating that bounds have non-negative coordinates (x, y >= 0)
- Checking that bounds have positive dimensions (w > 0, h > 0)
- Verifying bounds are positioned within desktop boundaries (x+w <= size[0], y+h <= size[1])
Example enhancement:
def test_get_desktop_usable_bounds(self): bounds = pygame.display.get_desktop_usable_bounds() sizes = pygame.display.get_desktop_sizes() self.assertIsInstance(bounds, list) self.assertEqual(len(bounds), len(sizes)) for i, bound in enumerate(bounds): self.assertIsInstance(bound, pygame.Rect) size = sizes[i] self.assertLessEqual(bound.w, size[0]) self.assertLessEqual(bound.h, size[1]) + # Optional: validate coordinates and dimensions + self.assertGreaterEqual(bound.x, 0) + self.assertGreaterEqual(bound.y, 0) + self.assertGreater(bound.w, 0) + self.assertGreater(bound.h, 0)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/display_test.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
test/display_test.py (2)
buildconfig/stubs/pygame/display.pyi (2)
get_desktop_usable_bounds(425-441)get_desktop_sizes(406-423)buildconfig/stubs/pygame/rect.pyi (1)
Rect(303-303)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: AMD64
- GitHub Check: x86
- GitHub Check: aarch64
- GitHub Check: x86_64
- GitHub Check: debug_coverage (ubuntu-24.04, 3.9.23)
- GitHub Check: i686
- GitHub Check: debug_coverage (ubuntu-24.04, 3.14.0rc1)
- GitHub Check: debug_coverage (ubuntu-24.04, 3.13.5)
- GitHub Check: Debian (Bookworm - 12) [ppc64le]
- GitHub Check: msys2 (clang64, clang-x86_64)
- GitHub Check: Debian (Bookworm - 12) [armv7]
- GitHub Check: msys2 (ucrt64, ucrt-x86_64)
- GitHub Check: Debian (Bookworm - 12) [armv6]
- GitHub Check: Debian (Bookworm - 12) [s390x]
- GitHub Check: build (windows-latest)
- GitHub Check: msys2 (mingw64, x86_64)
- GitHub Check: build (ubuntu-24.04)
- GitHub Check: build (macos-14)
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: dev-check
Yeah I agree, useful to manually maximize a window with it being aware of the taskbar or other things occupying space. Also thanks myre for pushing the memory fixes, have been out of the contributing loop during summer (github even signed me out...) |
|
This is more so coming from @Starbuck5, but pretend it's not so... or don't... anyway Here's the idea, So the API would look more like this (if, for example, you wanted a list of usable bounds) displays = pygame.display.get_displays()
desktop_usable_bounds = [d.usable_bounds for d in displays]
What all of this means for this PR:
These are required changes by the way 😁 |
|
@Matiiss so you think the backbone of the API should be implemented in this specific pull request? |
5 participants
As I said on discord,
pygame.display.get_desktop_sizesexist but doesn't take in consideration the taskbar or any other OS reserved space. The SDL function for it exists, so it makes sense to me to add it to display. It would be very tedious to do this in python cross platform without it. It's a differnt function from the sizes because it returns rects, but the return list uses the same logic so with the same index you get the full size and usable area for the same desktop. Also, it's alredy SDL3 compatible, so there will be no problems when display is ported. There is no conflict with window because this is one of the features that make sense for display, and doesn't concert any windows (display should only hold this functions)